From 2f2769b4e239964e7f88c2cd8746ac3b1925df9c Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Mon, 4 Aug 2014 10:50:32 -0700 Subject: [PATCH] Update the lockfile when deps are modified Closes #314 --- src/cargo/core/resolver.rs | 8 ++- src/cargo/core/source.rs | 3 +- src/cargo/ops/cargo_compile.rs | 87 +++++++++++++++++++++------ src/cargo/util/profile.rs | 2 +- tests/test_cargo_generate_lockfile.rs | 61 +++++++++++++++++++ 5 files changed, 140 insertions(+), 21 deletions(-) diff --git a/src/cargo/core/resolver.rs b/src/cargo/core/resolver.rs index 3cf4eaaed..b7e1a6b9e 100644 --- a/src/cargo/core/resolver.rs +++ b/src/cargo/core/resolver.rs @@ -193,13 +193,17 @@ fn encodable_package_id(id: &PackageId, root: &PackageId) -> EncodablePackageId impl Resolve { fn new(root: PackageId) -> Resolve { - Resolve { graph: Graph::new(), root: root } + let mut g = Graph::new(); + g.add(root.clone(), []); + Resolve { graph: g, root: root } } pub fn iter(&self) -> Nodes { self.graph.iter() } + pub fn root(&self) -> &PackageId { &self.root } + pub fn deps(&self, pkg: &PackageId) -> Option> { self.graph.edges(pkg) } @@ -406,7 +410,7 @@ mod test { pub fn test_resolving_empty_dependency_list() { let res = resolve(&pkg_id("root"), [], &mut registry(vec!())).unwrap(); - assert_that(&res, equal_to(&names([]))); + assert_that(&res, equal_to(&names(["root"]))); } #[test] diff --git a/src/cargo/core/source.rs b/src/cargo/core/source.rs index c186a628f..a3a28e61a 100644 --- a/src/cargo/core/source.rs +++ b/src/cargo/core/source.rs @@ -184,8 +184,9 @@ impl PartialEq for SourceId { if self.location == other.location { return true } match (&self.kind, &other.kind, &self.location, &other.location) { - (&GitKind(..), &GitKind(..), + (&GitKind(ref ref1), &GitKind(ref ref2), &Remote(ref u1), &Remote(ref u2)) => { + ref1 == ref2 && git::canonicalize_url(u1.to_string().as_slice()) == git::canonicalize_url(u2.to_string().as_slice()) } diff --git a/src/cargo/ops/cargo_compile.rs b/src/cargo/ops/cargo_compile.rs index c6f87c923..bdb71707e 100644 --- a/src/cargo/ops/cargo_compile.rs +++ b/src/cargo/ops/cargo_compile.rs @@ -23,10 +23,11 @@ //! use std::os; -use std::collections::HashMap; +use std::collections::{HashMap, HashSet}; use core::registry::PackageRegistry; use core::{MultiShell, Source, SourceId, PackageSet, Target, PackageId}; +use core::{Package, Summary, Resolve}; use core::resolver; use ops; use sources::{PathSource}; @@ -76,26 +77,18 @@ pub fn compile(manifest_path: &Path, let source_id = package.get_package_id().get_source_id(); let mut config = try!(Config::new(*shell, update, jobs, target.clone())); - let mut registry = PackageRegistry::new(&mut config); - let resolved = match try!(ops::load_lockfile(&lockfile, source_id)) { - Some(r) => { - try!(registry.add_sources(r.iter().map(|p| { - p.get_source_id().clone() - }).collect())); - r - } - None => { - try!(registry.add_sources(package.get_source_ids())); - try!(resolver::resolve(package.get_package_id(), - package.get_dependencies(), - &mut registry)) - } - }; + match try!(ops::load_lockfile(&lockfile, source_id)) { + Some(r) => try!(add_lockfile_sources(&mut registry, &package, &r)), + None => try!(registry.add_sources(package.get_source_ids())), + } - try!(registry.add_overrides(override_ids)); + let resolved = try!(resolver::resolve(package.get_package_id(), + package.get_dependencies(), + &mut registry)); + try!(registry.add_overrides(override_ids)); let resolved_with_overrides = try!(resolver::resolve(package.get_package_id(), package.get_dependencies(), @@ -208,3 +201,63 @@ fn scrape_target_config(config: &mut Config, Ok(()) } + +/// When a lockfile is present, we want to keep as many dependencies at their +/// original revision as possible. We need to account, however, for +/// modifications to the manifest in terms of modifying, adding, or deleting +/// dependencies. +/// +/// This method will add any appropriate sources from the lockfile into the +/// registry, and add all other sources from the root package to the registry. +/// Any dependency which has not been modified has its source added to the +/// registry (to retain the precise field if possible). Any dependency which +/// *has* changed has its source id listed in the manifest added and all of its +/// transitive dependencies are blacklisted to not be added from the lockfile. +/// +/// TODO: this won't work too well for registry-based packages, but we don't +/// have many of those anyway so we should be ok for now. +fn add_lockfile_sources(registry: &mut PackageRegistry, + root: &Package, + resolve: &Resolve) -> CargoResult<()> { + let deps = resolve.deps(root.get_package_id()).move_iter().flat_map(|deps| { + deps.map(|d| (d.get_name(), d)) + }).collect::>(); + + let mut sources = vec![root.get_package_id().get_source_id().clone()]; + let mut to_avoid = HashSet::new(); + let mut to_add = HashSet::new(); + for dep in root.get_dependencies().iter() { + match deps.find(&dep.get_name()) { + Some(&lockfile_dep) => { + let summary = Summary::new(lockfile_dep, []); + if dep.matches(&summary) { + fill_with_deps(resolve, lockfile_dep, &mut to_add); + } else { + fill_with_deps(resolve, lockfile_dep, &mut to_avoid); + sources.push(dep.get_source_id().clone()); + } + } + None => sources.push(dep.get_source_id().clone()), + } + } + + // Only afterward once we know the entire blacklist are the lockfile + // sources added. + for addition in to_add.iter() { + if !to_avoid.contains(addition) { + sources.push(addition.get_source_id().clone()); + } + } + + return registry.add_sources(sources); + + fn fill_with_deps<'a>(resolve: &'a Resolve, dep: &'a PackageId, + set: &mut HashSet<&'a PackageId>) { + if !set.insert(dep) { return } + for mut deps in resolve.deps(dep).move_iter() { + for dep in deps { + fill_with_deps(resolve, dep, set); + } + } + } +} diff --git a/src/cargo/util/profile.rs b/src/cargo/util/profile.rs index 11e43e1f5..20f9070c0 100644 --- a/src/cargo/util/profile.rs +++ b/src/cargo/util/profile.rs @@ -54,9 +54,9 @@ impl Drop for Profiler { print(0, msgs.as_slice()); } else { msgs.push((stack.len(), end - start, msg)); + messages.replace(Some(msgs)); } profile_stack.replace(Some(stack)); - messages.replace(Some(msgs)); } } diff --git a/tests/test_cargo_generate_lockfile.rs b/tests/test_cargo_generate_lockfile.rs index c9eed05a4..4e7823136 100644 --- a/tests/test_cargo_generate_lockfile.rs +++ b/tests/test_cargo_generate_lockfile.rs @@ -33,3 +33,64 @@ test!(ignores_carriage_return { execs().with_status(0)); assert_eq!(lockfile.stat().assert().modified, mtime); }) + +test!(adding_and_removing_packages { + let p = project("foo") + .file("Cargo.toml", r#" + [package] + name = "foo" + authors = [] + version = "0.0.1" + "#) + .file("src/main.rs", "fn main() {}"); + + assert_that(p.cargo_process("cargo-generate-lockfile"), + execs().with_status(0)); + + let lockfile = p.root().join("Cargo.lock"); + let toml = p.root().join("Cargo.toml"); + let lock1 = File::open(&lockfile).read_to_string().assert(); + + // add a dep + File::create(&toml).write_str(r#" + [package] + name = "foo" + authors = [] + version = "0.0.1" + + [dependencies] + bar = "0.5.0" + "#).assert(); + assert_that(p.process(cargo_dir().join("cargo-generate-lockfile")), + execs().with_status(0)); + let lock2 = File::open(&lockfile).read_to_string().assert(); + assert!(lock1 != lock2); + + // change the dep + File::create(&toml).write_str(r#" + [package] + name = "foo" + authors = [] + version = "0.0.1" + + [dependencies] + bar = "0.2.0" + "#).assert(); + assert_that(p.process(cargo_dir().join("cargo-generate-lockfile")), + execs().with_status(0)); + let lock3 = File::open(&lockfile).read_to_string().assert(); + assert!(lock1 != lock3); + assert!(lock2 != lock3); + + // remove the dep + File::create(&toml).write_str(r#" + [package] + name = "foo" + authors = [] + version = "0.0.1" + "#).assert(); + assert_that(p.process(cargo_dir().join("cargo-generate-lockfile")), + execs().with_status(0)); + let lock4 = File::open(&lockfile).read_to_string().assert(); + assert_eq!(lock1, lock4); +}) -- 2.30.2